Fix DimensionValue props crashing ViewManagers on Android - #58039
Closed
dennytosp wants to merge 1 commit into
Closed
Fix DimensionValue props crashing ViewManagers on Android#58039dennytosp wants to merge 1 commit into
dennytosp wants to merge 1 commit into
Conversation
Codegen maps a `DimensionValue` prop to the `DimensionPrimitive` reserved
type and emits a `@Nullable YogaValue` setter on the generated ViewManager
interface. If the ViewManager implements that setter with `@ReactProp`,
the app crashes while collecting view manager constants:
RuntimeException: Unrecognized type: class com.facebook.yoga.YogaValue
for method: MyNativeViewManager#setMarginBack
`ViewManagersPropertyCache.createPropSetter` has no branch for
`YogaValue`, so it falls through to the `else` and throws.
The conversion already exists: `DimensionPropConverter` handles the three
shapes a dimension arrives in (null, a Double in points, a String such as
"100%") and is unit tested. It was only ever wired into generated
delegate code, never into the `@ReactProp` path — unlike
`ColorPropConverter`, which is used by both.
Add the missing `DimensionPropSetter`, modelled on `ColorPropSetter`,
delegating to that converter. It reports `mixed` as its prop type since a
dimension may be a number or a string, matching how colors are reported.
This is the same class of gap as the `@Nullable Float` one. With both
closed, every Java type `GeneratePropsJavaInterface` can emit is bindable
through `@ReactProp`.
|
@javache has imported this pull request. If you are a Meta employee, you can view this in D116911179. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary:
A Fabric component whose spec declares a
DimensionValueprop:crashes the app while React Native is collecting view manager constants, as soon
as the
ViewManagerimplements that prop with@ReactProp:Root cause
Codegen maps
DimensionValueto theDimensionPrimitivereserved type and emitsa boxed
@Nullable YogaValuesetter on the generated ViewManager interface — thisis covered by a committed snapshot:
ViewManagersPropertyCache.createPropSetterhas no branch forYogaValue, so itfalls through to the
elseand throws.The conversion logic already exists —
DimensionPropConverterhandles exactly thethree shapes a
DimensionValuearrives in (null, aDoublein points, aStringlike
"100%") and has its own unit tests. But it is only referenced by generateddelegate code (
GeneratePropsJavaDelegate.js), never by the@ReactProppath.Compare
ColorPropConverter, which is wired into both paths — that asymmetry isthe bug.
This adds the missing
DimensionPropSetter, modelled onColorPropSetter, whichdelegates wholesale to the existing converter. It reports
mixedas its prop typebecause a dimension may be a number or a string, matching how colors are reported.
Note the blast radius:
getNativePropSettersForViewManagerClassbuilds the settermap for an entire ViewManager class in one pass, so a single unbindable prop takes
down every other prop on that manager, and
createConstantsForViewManagerreadsviewManager.nativePropsunconditionally. That is why this surfaces as a startupcrash rather than a failure when the prop is first set.
This is the same class of gap as #55350 (
@Nullable Float). With both fixed, everyJava type
GeneratePropsJavaInterfacecan emit is bindable through@ReactProp.Changelog:
[ANDROID] [FIXED] - Fix
RuntimeException: Unrecognized type: class com.facebook.yoga.YogaValuewhen aViewManagerimplements aDimensionValueprop with@ReactPropTest Plan:
Added
testDimensionSetterandtestFailToUpdateDimensionPropWithArraytoReactPropAnnotationSetterTest, plus adimensionPropon the ViewManager undertest. They drive
viewManager.updateProperties(...)— the sameFallbackViewManagerSetter->getNativePropSettersForViewManagerClass->createPropSetterpath as the crash — and assert the setter receivesYogaValue(10.5f, POINT)for a number,YogaValue(100f, PERCENT)for"100%",and
nullfornull, and that an unsupported value still surfaces as aJSApplicationIllegalArgumentExceptionrather than escaping raw.Reverting only
ViewManagersPropertyCache.ktand keeping the new tests reproducesthe crash:
All 19 fail rather than only the two new ones, for the reason described above: the
setter map is built for the whole ViewManager class at once.
Formatting —
./gradlew ktfmtFormatleaves both changed files byte-identical.ViewManagersPropertyCacheis aninternal objectandDimensionPropSetteris aprivate class, soReactAndroid.apiis unchanged.